I can't take a look now. But we should double check the specification for OpenGL 3.3 and OpenGL ES 3.0 and see if the return value for GL_MAX_VIEWPORT_DIMS is a 32 but value or 64 but value. That will inform whether we use the normal variant or the 32 but variant of getIntegerV. You can find that information in the PDF copy of the specification.

From https://registry.khronos.org/OpenGL/specs/es/3.0/es_spec_3.0.pdf

Get value MAX VIEWPORT DIMS
Type 2 × Z+ (Non-negative integer or enumerated token value)
Get Command GetIntegerv
Description Maximum viewport dimensions

I think we're safe with 32 bit GetIntegerv.

So I more or less reverted most of the changes from #80909

From the documentation, it seems that actually MAX_UNIFORM_BLOCK_SIZE should be queried as 64 bit. The remaining variables remain GLint (32-bit).

The total amount of buffer object storage available for any given uniform block is subject to an implementation-dependent limit. The maximum amount of avail- able space, in basic machine units, can be queried by calling GetInteger64v with the constant MAX_UNIFORM_BLOCK_SIZE. If the amount of storage required for a uniform block exceeds this limit, a program may fail to link.

buffer size

By the way, I checked where the references to config->max_uniform_buffer_size are located and I have a question, what to do with the following?

buffer_size is type of uint32_t and max_uniform_buffer_size is 64 bit (since Aug 28, 2023) and sometimes can be INT_MAX + 1 #80909 (comment) 🤔

viewport dims

I changed the reading of GL_MAX_VIEWPORT_DIMS in rasterizer_gles3.cpp to use the previously retrieved value from Config. This gave rise to two topics:

https://github.com/godotengine/godot/blob/20d5b6c1117579a3187bb2a80f620b40d2dc5ce5/drivers/gles3/rasterizer_gles3.cpp#L396-L406

CC: @RandomShaper #90913

1. As I understand it, referencing a singleton from Utilities will not cause threading issues here?

2. When the game screen is smaller than the window, this code creates a viewport of size GL_MAX_VIEWPORT_DIMS in each image frame and fills it with black. Isn't this overkill? Using my Firefox / Windows / GTX960 example, this means creating a black 32767x 32767 rectangle every frame. 💣

shader_gles3

For consistency and DRY, is it safe to add a reference to the singleton from Config in shader_gles3.cpp?

By the way, I checked where the references to config->max_uniform_buffer_size are located and I have a question, what to do with the following?

godot/drivers/gles3/storage/material_storage.cpp

Lines 1111 to 1115 in 5833f59

	global_shader_uniforms.buffer_size = MAX(4096, (int)GLOBAL_GET("rendering/limits/global_shader_variables/buffer_size"));
	if (global_shader_uniforms.buffer_size > uint32_t(Config::get_singleton()->max_uniform_buffer_size)) {
	global_shader_uniforms.buffer_size = uint32_t(Config::get_singleton()->max_uniform_buffer_size);
	WARN_PRINT("Project setting \"rendering/limits/global_shader_variables/buffer_size\" exceeds maximum uniform buffer size of: " + itos(Config::get_singleton()->max_uniform_buffer_size));
	}

buffer_size is type of uint32_t and max_uniform_buffer_size is 64 bit (since Aug 28, 2023) and sometimes can be INT_MAX + 1 #80909 (comment) 🤔

We should probably clamp max_uniform_buffer_size to a reasonable size in config.cpp . Maybe 1048576 (1 mb). Realistically almost every GPU only supports a maximum of 65536. I suspect many drivers only support 65536 even when they advertise support for more

viewport dims

I changed the reading of GL_MAX_VIEWPORT_DIMS in rasterizer_gles3.cpp to use the previously retrieved value from Config. This gave rise to two topics:

1. As I understand it, referencing a singleton from Utilities will not cause threading issues here?

That's right. Reading from the Window is what caused the issue, reading from Config/Utilities is fine.

2. When the game screen is smaller than the window, this code creates a viewport of size GL_MAX_VIEWPORT_DIMS in each image frame and fills it with black. Isn't this overkill? Using my Firefox / Windows / GTX960 example, this means creating a black 32767x 32767 rectangle every frame. 💣

Its not a problem. This just sets the Viewport size. The actual drawing will be limited to the size of the framebuffer/texture.

shader_gles3

For consistency and DRY, is it safe to add a reference to the singleton from Config in shader_gles3.cpp?

godot/drivers/gles3/shader_gles3.cpp

Line 804 in 5833f59

glGetInteger64v(GL_MAX_TEXTURE_IMAGE_UNITS, &max_image_units);

Yes.

OK, how does it look now?

I've added CLAMP in config.cpp to max_uniform_buffer_size (GL_MAX_UNIFORM_BLOCK_SIZE) from minimal 16K (according to khronos.org) to maximum 1MB.

max_uniform_buffer_size = CLAMP(max_uniform_buffer_size, 16384, 1048576);

💣 One more thing, as I am assuming, initialize in shader_gles3.cpp is performed in threads (in my case it runs 12 times). can I be 100% sure that this will always happen already after Config initialisation?

💣 One more thing, as I am assuming, initialize in shader_gles3.cpp is performed in threads (in my case it runs 12 times). can I be 100% sure that this will always happen already after Config initialisation?

Yes, Config is initialized before any shaders. I don't think any of the shaders are initialized from threads by the way. I'm fairly certain all shader initialization happens on the rendering thread and at initialization time.

Ah, yes, my mistake. ShaderGLES3::initialize is run multiple times because it is used by different modules called from RasterizerGLES3::RasterizerGLES3.

Thanks!